refactor(engine): type capture failures#2161
Conversation
5e82fec to
fc61671
Compare
131d9c1 to
67f5483
Compare
fc61671 to
b683ddb
Compare
7935b9d to
bd29456
Compare
b683ddb to
4e86382
Compare
bd29456 to
e10f87a
Compare
1a7d917 to
7dd4686
Compare
e10f87a to
9e02fbc
Compare
7dd4686 to
8733675
Compare
533dc78 to
5d9ca34
Compare
0c42de6 to
f095fb4
Compare
5d9ca34 to
2d9a934
Compare
f095fb4 to
eda0bb1
Compare
2d9a934 to
d564f84
Compare
eda0bb1 to
c0bd5b9
Compare
d564f84 to
8c275ea
Compare
c0bd5b9 to
571341a
Compare
8c275ea to
01d355d
Compare
571341a to
606e05b
Compare
01d355d to
26d4d2e
Compare
606e05b to
6cf4221
Compare
miguel-heygen
left a comment
There was a problem hiding this comment.
The typed failure boundary is a strong improvement overall: cancellation is authoritative, worker diagnostics stay immutable and attached to the representative failure, and the first fatal worker now stops sibling capture without hiding cleanup. The linear same-line I/O scan and 100k-character adversarial test also close the CodeQL finding without broadening classification.
One retry regression remains:
- [P1] Preserve retryability for the drawElement worker watchdog timeout —
packages/engine/src/services/captureFailure.ts:57-64enumerates protocol timeout strings but omits the engine's owndrawElement worker encode timed out (frame N)failure. That error is produced by the 30-second lost-message/OOM-killed-worker watchdog atpackages/engine/src/services/drawElementService.ts:674-684(and the batch sibling at:901-906), then propagates throughcaptureFrameRange's awaitedencodeResult. On this exact head,classifyCaptureFailure(new Error("drawElement worker encode timed out (frame 42)"))returnsauthoring, soparallelCoordinator.ts:594-598treats it as fatal and aborts peers, whilerenderOrchestrator.ts:817-821refuses the worker-halving retry. Before this PR, the broadtimed outrecovery matcher covered this concrete failure. Please classify this exact watchdog failure as non-fatal/retryable and pin both classification and recovery behavior with a regression test.
Exact head reviewed: 9ad957786d14040f72b87ac2f72ac59a028e6f90. All exact-head required checks are green; this is a code-contract blocker rather than a CI failure.
Verdict: REQUEST CHANGES
Reasoning: The typed model and peer-abort mechanism are sound, but a real engine-generated worker timeout regresses from retryable to fatal, causing avoidable render failures instead of the prior recovery path.
— Magi
|
Fixed Magi’s P1 on exact head
Fresh exact-head CI is running. This is a substantive head change and requires a fresh exact-head review after all required checks are green. |

What
Replace message-parsed capture retry policy with typed CaptureFailure values and peer abort.
Why
Stringified worker errors lost causes and let sibling workers continue after fatal authoring failures.
How
Classify cancellation, browser, protocol, memory, verification, authoring, and IO failures; preserve diagnostics and abort peers on the first fatal kind.
Test plan